Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Metapackages #859

Merged
merged 218 commits into from
May 31, 2023
Merged

Metapackages #859

merged 218 commits into from
May 31, 2023

Conversation

perazz
Copy link
Contributor

@perazz perazz commented Mar 24, 2023

This PR aims to address #853 #161 #855. The first 4 metapackages (OpenMP, stdlib, MPI, minpack) are implemented.
Documentation at fortran-lang/fpm-docs#108.

Usage

Metapackages are supported via a keyword in the manifest's dependencies section like it was suggested in #161:

[dependencies]
openmp="*"

Currently, we only support the wildcard ("*"), meaning "any" versions. In the future, this can be extended with constraints.
example programs are given in example_packages/metapackage_* folders, and they're all run by the new CI.

Implementation status

  • openmp
  • fortran-lang stdlib
  • fortran-lang minpack
  • MPI: MacOS/x86_64 + GCC + OpenMPI (via brew or mamba)
  • MPI: Windows/x86_64 + MinGW + MSMPI (via msys2)
  • MPI: MacOS/x86_64 + GCC + MPICH (via brew or mamba)
  • MPI: Linux/x86_64 + GCC + OpenMPI (via apt or mamba)
  • MPI: Linux/x86_64 + GCC + MPICH (via apt or mamba)
  • MPI: Linux/x86_64 + Intel + IntelMPI (via apt)

Summary

  • metapackage_t is a project handler that adds flags, dependencies and features to a package to fit the metapackage requirement(s) without messing up the codebase.
  • New CI action meta.yml only runs when metapackages are added/modified
  • Specific implementations are currently in fpm_meta.f90 for now, will be moved out to individual files next.

@perazz
Copy link
Contributor Author

perazz commented Mar 24, 2023

@perazz
Copy link
Contributor Author

perazz commented Mar 30, 2023

I've added a metapackage for stdlib. So now we have two:

[build]
openmp = "*"
stdlib = "*"

TBD:
what should happen if a package requires another package with different metapackage requirements? this will be important especially for openmp/mpi/opencl...

E.g., my package has no openmp but one of its dependencies has it. Should we just propagate openmp = "*" all the way up or stop the build saying that openmp should be enabled? I would go for option no. 2, it forces the user to know what they're doing.

@perazz
Copy link
Contributor Author

perazz commented May 11, 2023

Appreciate your help @arteevraina, thank you. I believe all your comments are addressed now.

@perazz
Copy link
Contributor Author

perazz commented May 11, 2023

The Windows build sometimes segfaults in the CI (not on my local machine), so, I'm investigating. I suspect it has to do with the fact that the CI uses the msys shell.

@perazz
Copy link
Contributor Author

perazz commented May 29, 2023

Thank you @arteevraina. If there are no more comments from @minhqdao @henilp105 and the other reviewers, I will merge within a few days.

@perazz
Copy link
Contributor Author

perazz commented May 30, 2023

Ok, looks like it's good enough to merge.

@perazz perazz merged commit 2e1e486 into fortran-lang:main May 31, 2023
@Carltoffel
Copy link
Member

E.g., my package has no openmp but one of its dependencies has it. Should we just propagate openmp = "*" all the way up or stop the build saying that openmp should be enabled?

I guess this is the reason I get Unable to find source for module dependency: "stdlib_logger" because it is used in a dependency and I don't directly use/import stdlib in my project?

@perazz
Copy link
Contributor Author

perazz commented Jun 3, 2023

Thanks @Carltoffel, good point that I agree it needs to be addressed too. Can I ask you to open a new issue on it for discussion?

@Carltoffel
Copy link
Member

Can I ask you to open a new issue on it for discussion?

Of course, you can, but I don't quite get what the topic exactly should be. Do you want to discuss a better error message? Or do you want to have a better discussion on how to handle nested dependencies of metapackages?

@perazz
Copy link
Contributor Author

perazz commented Jun 5, 2023

Correct - thank you! I mean about handling nested dependencies with metapackages. Maybe having an issue for it will draw more attention/discussion.

@Carltoffel
Copy link
Member

When I use the MPI metapackage with use mpi, I also have to use build.external-modules = ["mpi"]. Is this behaviour intended? Otherwise, I get:

 + which mpiexec
/usr/bin/mpiexec
<ERROR> *cmd_run* Targets error: Unable to find source for module dependency: "mpi" used by "app/main.f90"

In the example_packages directory, import mpif.h is used, which circumvents this. Which method is preferred?

@perazz
Copy link
Contributor Author

perazz commented Jun 6, 2023

@Carltoffel, thank you for testing MPI.
So far there is no checking of MPI Fortran modules as their support is inconsistent across the implementations.
However, this should not prevent the user from using them.
As a first step, I will open a PR to automatically add mpi and mpi_f08 to the list of external modules.

@perazz perazz mentioned this pull request Jun 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants